Skip to content

feat(files): export markdown as zip with embedded images#4413

Merged
waleedlatif1 merged 13 commits intostagingfrom
fix/links
May 3, 2026
Merged

feat(files): export markdown as zip with embedded images#4413
waleedlatif1 merged 13 commits intostagingfrom
fix/links

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Adds GET /api/files/export/[id] — when the target file is markdown, downloads all embedded workspace images, rewrites ![alt](/api/files/view/{id}) refs to ./assets/{filename}, and returns a self-contained zip
  • Non-markdown files redirect to the existing serve path (no behaviour change)
  • Updates client downloadWorkspaceFile to route markdown files through the export endpoint automatically; falls back to raw serve for files without an id
  • Filename sanitization (path.basename + strip ", \, control chars) prevents header injection and zip path traversal
  • Storage context cast corrected to StorageContext (was forced to 'workspace', breaking mothership files)
  • Filename collision deduplication: appends first 8 chars of UUID when two images share an original name
  • Parallel image fetch capped at 50; deduplication runs serially after fetch to avoid race on shared Set
  • Route baseline updated from 722 → 723

Type of Change

  • New feature

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 3, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment May 3, 2026 3:01am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 3, 2026

PR Summary

Medium Risk
Adds a new authenticated export endpoint that rewrites and bundles file contents into a generated ZIP, plus broad refactors to file download plumbing; bugs here could impact download correctness/performance and access control. Additional workflow-lock UX tweaks are low risk but touch interactive canvas/notification behavior.

Overview
Adds GET /api/files/export/[id] to export Markdown files as a self-contained ZIP by rewriting /api/files/view/{uuid} image links to local ./assets/* paths and embedding up to 50 accessible images; non-Markdown exports redirect to the existing serve URL. Filenames are sanitized and deduplicated to avoid traversal/header issues and collisions.

Refactors workspace file reads by renaming server-side downloadWorkspaceFile to fetchWorkspaceFileBuffer and updating API routes/tools/tests accordingly. Client-side downloads are moved to a new triggerFileDownload helper which routes Markdown downloads through the export endpoint.

Tweaks workflow lock UX: unlock notifications no longer auto-dismiss and don’t disappear when clicked, locked canvases now surface an info notification on drop attempts, admin unlock controls are disabled while read-only, and locked canvas opacity is adjusted.

Reviewed by Cursor Bugbot for commit 76a07aa. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 3, 2026

Greptile Summary

This PR adds a GET /api/files/export/[id] endpoint that bundles a markdown file and its embedded workspace images into a self-contained zip, rewriting /api/files/view/{id} references to relative ./assets/{filename} paths. It also renames downloadWorkspaceFilefetchWorkspaceFileBuffer (server-side buffer download) and replaces the client downloadWorkspaceFile utility with a new triggerFileDownload helper that routes markdown through the export endpoint automatically. Additional changes improve locked-workflow UX and fix the StorageContext cast for mothership files.

Confidence Score: 5/5

Safe to merge — only P2 style/robustness findings; core export logic, auth, and filename sanitisation are sound.

All findings are P2: incomplete control-character stripping in Content-Disposition (functional but non-spec) and missing filename* RFC 5987 encoding for non-ASCII names (browser-tolerated). No P0/P1 issues found. The access-check, deduplication, zip-slip prevention, and rename refactor are all correct.

apps/sim/app/api/files/export/[id]/route.ts — two minor header-encoding improvements suggested.

Important Files Changed

Filename Overview
apps/sim/app/api/files/export/[id]/route.ts New export endpoint: builds a zip with sanitised filenames, parallel image fetch, deduplication — two P2 header-sanitisation improvements noted.
apps/sim/lib/uploads/client/download.ts New client helper: routes markdown files through /api/files/export, non-markdown through serve; WorkspaceFileRecord.id is non-optional so record.id usage is safe.
apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts Rename-only: downloadWorkspaceFile → fetchWorkspaceFileBuffer with identical implementation; all call-sites updated consistently.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx Locked-workflow UX improvements: uses isAdmin from workspacePermissions, adds onDropLocked handler, prevents canAdmin from controlling lock UI while workflow is locked.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/notifications/notifications.tsx Intentional UX change: unlock-workflow notifications are no longer auto-dismissed and return early to prevent manual dismissal after click.
apps/sim/lib/api/contracts/storage-transfer.ts Adds fileExportContract and fileExportParamsSchema for the new export route; consistent with existing file contracts.

Sequence Diagram

sequenceDiagram
    participant C as Client (triggerFileDownload)
    participant E as GET /api/files/export/[id]
    participant S as GET /api/files/serve/…
    participant DB as File Metadata DB
    participant Stor as Storage Service

    C->>C: isMarkdown(record)?
    alt markdown
        C->>E: GET /api/files/export/{id}
        E->>E: checkSessionOrInternalAuth
        E->>DB: getFileMetadataById(id)
        E->>E: verifyFileAccess
        E->>E: isMarkdown(originalName, contentType)?
        alt not markdown on server
            E-->>S: 302 redirect to serve path
        else markdown
            E->>Stor: downloadFile(key, context)
            E->>E: extract imageIds via VIEW_URL_RE (up to 50)
            par fetch all images
                E->>DB: getFileMetadataById(imageId)
                E->>Stor: downloadFile(imgKey, context)
            end
            E->>E: deduplicate filenames, rewrite md refs
            E->>E: zip.file(mdName, content) + assets/
            E-->>C: 200 application/zip
        end
    else non-markdown
        C->>S: GET /api/files/serve/{key}?context=workspace
        S-->>C: raw file
    end
    C->>C: createObjectURL → anchor click → download
Loading

Reviews (5): Last reviewed commit: "fix(tests): update mocks for fetchWorksp..." | Re-trigger Greptile

Comment thread apps/sim/app/api/files/export/[id]/route.ts
Comment thread apps/sim/app/api/files/export/[id]/route.ts
Comment thread apps/sim/app/api/files/export/[id]/route.ts
Comment thread apps/sim/app/api/files/export/[id]/route.ts Outdated
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/api/files/export/[id]/route.ts
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/uploads/utils/file-utils.ts Outdated
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx Outdated
…ale lock notification

When workspacePermissions loads asynchronously, prevCanAdminRef (which only
tracked effectivePermissions.canAdmin) would not detect the change, causing
the early-return guard to skip rebuilding the notification with the correct
unlock-button visibility. Track the same resolved value (workspacePermissions
?.viewer?.isAdmin ?? effectivePermissions.canAdmin) that is actually used to
build the notification.
… client download to uploads/client/download.ts as triggerFileDownload
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 76a07aa. Configure here.

@waleedlatif1 waleedlatif1 merged commit a479e88 into staging May 3, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/links branch May 3, 2026 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant